feat: support clojure.core/get arity 3#87
Open
phrohdoh wants to merge 1 commit intoclojure-rs:masterfrom
phrohdoh:clojure.core/get,arity-3,not-found
Open
feat: support clojure.core/get arity 3#87phrohdoh wants to merge 1 commit intoclojure-rs:masterfrom phrohdoh:clojure.core/get,arity-3,not-found
clojure.core/get arity 3#87phrohdoh wants to merge 1 commit intoclojure-rs:masterfrom
phrohdoh:clojure.core/get,arity-3,not-found
Conversation
https://clojure.github.io/clojure/clojure.core-api.html#clojure.core/get > Returns the value mapped to key, not-found or nil if key not present. (get map key) ;; supported prior to this commit (get map key not-found) ;; supported as of this commit
phrohdoh
commented
Jul 15, 2021
| let map = persistent_list_map!(map_entry!("x", "v")); | ||
| let key = Keyword::intern("k").to_rc_value(); | ||
| let default = Keyword::intern("not-found").to_rc_value(); | ||
| let val: Rc<Value> = map.get_with_default(&key, &default); |
Contributor
Author
There was a problem hiding this comment.
I did not intend to leave these type annotations in.
- remove type annotations in
get_with_defaultandget_with_default_emptytests
Contributor
Author
|
See phrohdoh/cljrs@ceb3f9e for changes building on top of this PR to support map and keyword application default values: ({:k :v} :x) ;; => nil
({:k :v} :x :not-found) ;; => :not-found
(:x {:k :v}) ;; => nil
(:x {:k :v} :not-found) ;; => :not-found |
Tko1
approved these changes
Apr 18, 2023
| pmap.get_with_default(key, not_found) | ||
| } else { | ||
| pmap.get(key) | ||
| }.to_value(); |
Member
There was a problem hiding this comment.
Last part I'd like to add then is a test for GetFn (including one covering the original 2 arg'd GetFn we never wrote)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://clojure.github.io/clojure/clojure.core-api.html#clojure.core/get
Instead of adding a
not_found: Option<&Rc<Value>toIPersistentMap::get(to be threaded through all nested calls and a seemingly-arbitrary default to::getcallsites) I opted to continue the*_with_*convention; creating aget_with_defaultfunction.If merged, this would fix #86 (for maps, but I, perhaps incorrectly, recall reading that
getcan be applied to non-maps, though the linked docs certainly do not suggest so).